Skip to content

Comments

[4.0] com_menu - Change Featured Icon to Home Icon in#29016

Merged
infograf768 merged 5 commits intojoomla:4.0-devfrom
coolcat-creations:home-icon-for-menus
May 13, 2020
Merged

[4.0] com_menu - Change Featured Icon to Home Icon in#29016
infograf768 merged 5 commits intojoomla:4.0-devfrom
coolcat-creations:home-icon-for-menus

Conversation

@coolcat-creations
Copy link
Contributor

@coolcat-creations coolcat-creations commented May 9, 2020

Pull Request for Issue #28976

Summary of Changes

Changed the Default Icon in Menus to a home icon. If it has to be called home or default could be discussed in another issue. This PR makes a difference between the featured - action and the set menu item as default action. It also adds the possibility to pass the active and inactive iconclass suffix to the isDefault method.

Testing Instructions

apply patch
see if the home Icon on monolingual sites is now a home icon
the non-default items are for now a simple circle
on a multilingual website the home icon should be still a flag like before.

Expected result

grafik

Sidebar:
grafik

Actual result

grafik

@coolcat-creations coolcat-creations changed the title Change Featured Icon to Home Icon in com_menu [4.0] com_menu - Change Featured Icon to Home Icon in May 9, 2020
@infograf768
Copy link
Member

infograf768 commented May 10, 2020

Remark 1:

Size of icon is very different between sidemenu and Menu dasboard. Menu dashboard is better imho.
Screen Shot 2020-05-10 at 07 37 08

Remark 2:
Density of Yellow is very different between the featured star (for articles manager) and Home icon set to All languages in menu items manager. Articles manager star density is better imho.

Screen Shot 2020-05-10 at 07 41 57

Screen Shot 2020-05-10 at 07 43 33

The same colour is applied

color: var(--warning);
border-color: var(--warning);

The issue comes from

.disabled {
    opacity: .4;
}

These "issues" are unrelated to this patch but it would be good to solve them at the same time.

@coolcat-creations
Copy link
Contributor Author

coolcat-creations commented May 10, 2020

Yes, this PR is only about to change the type of the icon, not the color or size - for color we would need another class in the table header and for sizes it’s also not bound to only the home icon but the general interface. see later comment :-)

@adj9
Copy link

adj9 commented May 10, 2020

I have tested this item ✅ successfully on 109a7b2

Apply the PR and I've change the icon for menu.
I've do an article in person lang and see the icon should
image


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29016.

@coolcat-creations
Copy link
Contributor Author

I have tested this item ✅ successfully on 109a7b2

Apply the PR and I've change the icon for menu.
I've do an article in person lang and see the icon should
image
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29016.

Thank you!

@coolcat-creations
Copy link
Contributor Author

@adj9 @infograf768 I added the styling for the home item to not have the disabled class and added a "not-allowed" cursor. Can you retest?
For the icon sizes I think a separate PR would be better.

@infograf768
Copy link
Member

will do tomorrow.

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 38bc020


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29016.

@infograf768
Copy link
Member

@coolcat-creations

I suggest you also add a few lines above

  &.featured-disabled {
    opacity: 1;
  }

as this will now also solve density in Installed Languages and Template Styles for the default star colour.

@N6REJ
Copy link
Contributor

N6REJ commented May 11, 2020

@coolcat-creations

  &.disabled .icon-home {
    color: $state-warning-bg;
    border-color: $state-warning-bg;
  }

should be

&.disabled .fas.fa-home,
  &.disabled .icon-home {
    color: $state-warning-bg;
    border-color: $state-warning-bg;
  }

While below is preferred, I understand for b/c reasons that may not be possible at this time.

  &.disabled .fas.fa-home {
    color: $state-warning-bg;
    border-color: $state-warning-bg;
  }

@coolcat-creations
Copy link
Contributor Author

@N6REJ - I can't add code that is not applying yet, you would need to add this into your PR after mine is merged or me after yours is merged, but not before both are merged.

@coolcat-creations
Copy link
Contributor Author

coolcat-creations commented May 12, 2020

&.featured-disabled {
opacity: 1;
}

@infograf768 - May I create a seperate PR for that? I prefer not to mix up too much in case something needs to be looked up later or reverted.

@infograf768
Copy link
Member

@infograf768 - May I create a seperate PR for that? I prefer not to mix up too much in case something needs to be looked up later or reverted.

Sure, but don’t forget! 😺

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 0838c6d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29016.

1 similar comment
@Quy
Copy link
Contributor

Quy commented May 12, 2020

I have tested this item ✅ successfully on 0838c6d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29016.

@Quy Quy changed the title [4.0] com_menu - Change Featured Icon to Home Icon in [4.0] com_menu - Change Featured Icon to Home Icon in May 12, 2020
@Quy
Copy link
Contributor

Quy commented May 12, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29016.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 12, 2020
@infograf768 infograf768 merged commit 8cec1f3 into joomla:4.0-dev May 13, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 13, 2020
@infograf768
Copy link
Member

Tks

@infograf768 infograf768 added this to the Joomla 4.0 milestone May 13, 2020
@infograf768
Copy link
Member

@coolcat-creations
Now you can make the PR for #29016 (comment) ;)

rdeutz added a commit that referenced this pull request Jul 2, 2020
* several single icons found

* eye close

* eye

* admin page title

* $iconimage

* sub-menu

* icon-delete

* mapping non same named.

* reformat

* Update administrator/templates/atum/scss/pages/_com_privacy.scss

ty brian

Co-Authored-By: Brian Teeman <brian@teeman.net>

* refresh

* refresh

* remove

* featured

* featured

* column sort icon

* square

* $displaydata

* [class^='icon-']

* make fa-check green

* com_contacts featured

* more featured

* more publish/unpublish

* com_categories

* newsfeeds

* associations

* $iconStates

* fixed broken _icons.scss

* icon-ban-circle

* revert to icon-home per jm

* revert to icon-home per jm

* revert to icon-home per jm

* [4.0][RTL] CSS logical properties init

* Update _modals.scss

* use margin-inline

* Update _modals.scss

* Revert "Update _modals.scss"

This reverts commit eb0d8bb.

* Revert "use margin-inline"

This reverts commit 6b264dd.

* Use margin-inline-end in atum's _header.scss

* Fix CS from last commit in atum's _header.scss

* working on #28607

* working on #28607 ugh

* working on toolbar cross-reference missing from part 1

* icon-loop fix.

* finder fix

* more loop->sync changes

* more loop->sync changes

* puzzle -> puzzle-piece

* puzzle -> puzzle-piece

* joomla brand

* address->file-alt

* stack->copy

* added more to star, such as default

* removed blank line

* equalizer->sliders-h

* lightning->bolt

* checkin->check-square

* comments-2->comments

* power-cord->plug

* info-2->info-circle

* generic not fixed

* generic not fixed

* workaround for generic

* drone fix?

* puzzle->puzzle-piece

* per luca

* match button icon-class to toolbar

* chevron & publish

* publish/unpublish star.  NEEDS REVIEW!

* publish/unpublish star.  NEEDS REVIEW!

* few more icons

* more folders

* conflict fix.

* com_template->modals

* modifed jgrid to handle known active classes

* arrow-down3 -> caret-down

* icon-checkedout -> fa-lock

* icon-puzzle -> puzzle-piece

* trying to fix drone

* trying to fix drone

* trying to fix drone

* else if fix per quy

* Update layouts/joomla/button/iconclass.php

ty quy

Co-authored-by: Quy <quy@fluxbb.org>

* Update layouts/joomla/button/iconclass.php

ty quy

Co-authored-by: Quy <quy@fluxbb.org>

* code format fix per quy

* code formatting changes per @Quy

* code formatting changes

* articles toolbar icon

* remove space before 'fas'

* featured

* com_contact

* featured

* preview = icon-eye -> fas fa-eye

* workflows featured

* added mapping for $inactive class

* missing closing ?>

* Increase Opacity of disabled Featured Icons

* Change Icon Classes
Add Font Awesome Regular to Atum
Fix broken Icons after change

* Update build/media_source/com_categories/js/shared-categories-accordion.es6.js

ty sharky

Co-authored-by: SharkyKZ <sharkykz@gmail.com>

* Update build/media_source/system/js/fields/passwordview.es6.js

ty sharky.. I suck @ js.

Co-authored-by: SharkyKZ <sharkykz@gmail.com>

* Update build/media_source/system/js/fields/passwordview.es6.js

Co-authored-by: SharkyKZ <sharkykz@gmail.com>

* Update layouts/joomla/toolbar/title.php

Co-authored-by: Quy <quy@fluxbb.org>

* Update administrator/templates/atum/scss/template-rtl.scss

Co-authored-by: Quy <quy@fluxbb.org>

* pr #29016 compatibility fix.

* pr #29016 compatibility fix.
icon-default -> fa-home

* Update FeaturedButton.php

Add line break

* Update FeaturedButton.php

New line

* Update FeaturedButton.php

* Update FeaturedButton.php

* removed stupid package-lock.json

* b/c compatibility fix

* b/c compatibility fix

* Revert "b/c compatibility fix"

This reverts commit 10c4611

* "b/c compatibility fix"

* shelving

* shelving

* b/c compatability fix as found by jm.

* phpcs fix

* phpcs fix

* phpcs fix

* conflict fix.

* conflict fix.

* Revert "conflict fix."

This reverts commit ef96c0f

* Revert "conflict fix."

This reverts commit 4aa7349

* removed improperly kept 133-147

Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Co-authored-by: ciar4n <ciaran@joomla51.com>
Co-authored-by: Quy <quy@fluxbb.org>
Co-authored-by: coolcat-creations <mail@coolcat-creations.com>
Co-authored-by: SharkyKZ <sharkykz@gmail.com>
Co-authored-by: infograf768 <infografjms@gmail.com>
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
* Change Featured Icon to Home Icon in com_menu

* Change Icon class

* Change Default icon in sidebar menu too

* Different class for disabled Home item

* Update administrator/components/com_menus/tmpl/items/default.php

Co-authored-by: Quy <quy@fluxbb.org>

Co-authored-by: Quy <quy@fluxbb.org>
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
icon-default -> fa-home
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants